-
Notifications
You must be signed in to change notification settings - Fork 22
Add examples to docstrings for consumer methods #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add examples to docstrings for consumer methods #182
Conversation
rnowling-memphis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work @Adarsh-jaiss ! It's very clear and well formatted. Thank you! Some small changes.
memphis/consumer.py
Outdated
| await asyncio.sleep(1) | ||
| asyncio.run(main()) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the blank lines
memphis/consumer.py
Outdated
| await memphis.connect(host='localhost', username='user', password='pass') | ||
| consumer = await memphis.consumer(station_name='my_station', consumer_name='my_consumer', consumer_group='my_group') | ||
| consumer.set_context({'key': 'value'}) | ||
| consumer.consume(callback=message_handler) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can parameters be passed by name if they are not specified by keyword arguments? Might be safer to remove callback=.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see how I stated that could be confusing. I meant change it to this:
consumer = await memphis.consumer(station_name='my_station', consumer_name='my_consumer', consumer_group='my_group')
consumer.set_context({'key': 'value'})
consumer.consume(message_handler)- removed the first line and blank lines - Used the named parameters
rnowling-memphis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Adarsh-jaiss small miscommunication on my end -- I wasn't clear in my feedback. Sorry about that. Please see my note. Thanks!
memphis/consumer.py
Outdated
| await memphis.connect(host='localhost', username='user', password='pass') | ||
| consumer = await memphis.consumer(station_name='my_station', consumer_name='my_consumer', consumer_group='my_group') | ||
| consumer.set_context({'key': 'value'}) | ||
| consumer.consume(callback=message_handler) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see how I stated that could be confusing. I meant change it to this:
consumer = await memphis.consumer(station_name='my_station', consumer_name='my_consumer', consumer_group='my_group')
consumer.set_context({'key': 'value'})
consumer.consume(message_handler)
rnowling-memphis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @Adarsh-jaiss
The method
Consumer.fetch()has an example of how to use the method in its docstring. I have added a similar example to the docstring for Consumer.consume().Closed Issue Add examples to docstrings for consumer methods #181